This looks good to me outside of the minor comments. The way we're handling the not-always-set SchemaElement fields bums me out a bit, but if they can be counted on to always be -1 if not set it does make testing easier. Pretty open-minded about improving this aspect of the schema code generally

For the not-always-set SchemaElement fields, we probably need a way in SchemaElement to track if they were set (which we could also define as value == -1). Then we could decide consistently in the to-thrift-schema-conversion if we should write them out.

If the struct is zeroed out, you can use the element.__isset for this

Yes, we have that in Thrift. Probably I got it wrong in my last comment ;) What I'm missing is that we haven't got something similar to .__isset in PrimitiveNode to track if we have set one of length, precision, scale.

But to get this PR to a close: Should I only call element->__set_type_length if length != -1 or keep the code for as it is? (this is the only remaining issue here or am I missing something?)

Keeping things as is works for me right now, we can iterate on the optional field stuff later.

+1, merging